Skip to content

Fix IPsec + TLS 1.3 notebook + re-enable tests #3650

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 3 commits into from
Jun 20, 2022
Merged

Fix IPsec + TLS 1.3 notebook + re-enable tests #3650

merged 3 commits into from
Jun 20, 2022

Conversation

gpotter2
Copy link
Member

@gpotter2 gpotter2 commented Jun 18, 2022 •
edited
Loading

Various fixes to modules with crypto:

  • enables ~crypto-advanced in the tests on all platforms (previously not enabled because of cryptography availability):
    • TLS 1.3 had regressed
    • ipsec AES-CCM had regressed
  • fix various bugs in IPsec triggered by this change + add support for variable icv_size + ChaCha20Poly1305
  • finally fix the TLS 1.3 notebook -> new TLS 1.3 data provided by https://tls13.ulfheim.net/
  • fix a small bug in the TLS.dispatch_hook (-> TLS13), that would not be triggered if rcs was not commited (but the commit happens in TLS13). This was missed because we typically call TLS13(...) directly

fix #3418
fix #3416
fix #3073

@gpotter2 gpotter2 added the tls label Jun 18, 2022
@gpotter2 gpotter2 changed the title Update the TLS13 notebook to spec Fix IPsec + TLS 1.3 notebook + re-enable tests Jun 18, 2022
@gpotter2 gpotter2 added this to the 2.5.0 milestone Jun 18, 2022
@codecov
Copy link

codecov bot commented Jun 18, 2022 •
edited
Loading

Codecov Report

Merging #3650 (ce4a683) into master (31ee7b9) will increase coverage by 0.05%.
The diff coverage is 83.33%.

@@            Coverage Diff             @@
##           master    #3650      +/-   ##
==========================================
+ Coverage   86.21%   86.26%   +0.05%     
==========================================
  Files         288      288              
  Lines       65272    65662     +390     
==========================================
+ Hits        56273    56643     +370     
- Misses       8999     9019      +20
Impacted Files Coverage Δ
scapy/layers/tls/record.py 91.35% <0.00%> (-0.25%) ⬇️
scapy/layers/ipsec.py 88.95% <86.95%> (-0.69%) ⬇️
scapy/packet.py 73.39% <0.00%> (-8.70%) ⬇️
scapy/arch/windows/__init__.py 67.73% <0.00%> (-0.57%) ⬇️
scapy/contrib/automotive/ecu.py 94.33% <0.00%> (-0.34%) ⬇️
scapy/contrib/automotive/scanner/enumerator.py 91.96% <0.00%> (-0.30%) ⬇️
scapy/contrib/isotp/isotp_soft_socket.py 83.23% <0.00%> (-0.20%) ⬇️
scapy/layers/tls/extensions.py 83.22% <0.00%> (+0.62%) ⬆️
scapy/layers/l2.py 76.90% <0.00%> (+0.69%) ⬆️
... and 6 more

@gpotter2 gpotter2 force-pushed the tls13-notebook branch 2 times, most recently from 810e958 to a00c3f0 Compare June 18, 2022 23:24
@gpotter2
Copy link
Member Author

@p-l- @guedou Also ready for review ! (I am quite happy with what I did in this PR TBH :p)

p-l-
p-l- approved these changes Jun 20, 2022
View reviewed changes
Copy link
Member

@p-l- p-l- left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's huge and great work @gpotter2!

@p-l- p-l- merged commit 2c1747f into secdev:master Jun 20, 2022
@gpotter2 gpotter2 deleted the tls13-notebook branch June 20, 2022 08:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@p-l- p-l- p-l- approved these changes

Assignees
No one assigned
Labels
tls
Projects
None yet
Milestone
2.5.0
Development

Successfully merging this pull request may close these issues.

[Enhancement] ChaCha20Poly1305 support for IPSec Some TLS notebooks are outdated
2 participants
@gpotter2 @p-l-